Repository navigation
feat(rbac): enforce agent_use and llm_use, retire object_publish and dashboard_view (decisions 239, 247 to 249) - #4641
Open
rubenvdlinde wants to merge 5 commits into
Open
rubenvdlinde wants to merge 5 commits into
rubenvdlinde wants to merge 5 commits into
Conversation
…wner change special rights (decision 239) agent_use is checked when a chat message runs an agent and when an MCP session acts as one, read from the agent's organisation. llm_use is checked on every chat message before it is stored or a model is called, read from the conversation's organisation. Administrators pass; * means every signed-in user. A post-migration repair step writes * beside the stored groups once, so nobody is refused on upgrade. A change to any special right from a member who is not the owner or an administrator is refused with 403, and the editor's Special Rights tab is read-only for them. object_publish and dashboard_view name no action in the app today and stay unenforced; the questions are in the change's design.md. The docs no longer show the hasSpecialRight() call that does not exist.
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| test-l10n-parity | ❌ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ❌ | ✅ 173/173 | |||
| npm | ✅ | ✅ 648/648 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ✅ | ||||
| lockfile sync | ✅ | ||||
| PHPUnit | ❌ 26248/26250 · 25 skipped | ||||
| Newman | ✅ 714/714 | ||||
| Playwright | ⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it locally with npx playwright test, or from the Actions tab on a branch with no open pull request into development. |
||||
| Hydra gates | ❌ |
Quality workflow — 2026-10-15 10:43 UTC
Download the full PDF report from the workflow artifacts.
…rking without llm_use (decisions 247 to 249) Both rights were stored and shown but never enforced. They leave the organisation's defaults, the editor's Special Rights tab, the frontend entity, the admin-group backfill, the upgrade step and the docs. A stored block that still holds them loads and is served unchanged; nothing reads or writes them. The spec records that semantic search does not need llm_use.
…and read info.xml without libxml's file loader in the repair test The parity gate wants every required locale to carry "Everyone" and the read-only note. Under Nextcloud the libxml external entity loader refuses file paths, so simplexml_load_file() returned false in CI; the test now reads the file with file_get_contents(), as the other info.xml tests do.
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| test-l10n-parity | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 173/173 | |||
| npm | ✅ | ✅ 648/648 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ✅ | ||||
| lockfile sync | ✅ | ||||
| PHPUnit | ✅ 26248/26248 · 25 skipped | ||||
| Newman | ✅ 714/714 | ||||
| Playwright | ⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it locally with npx playwright test, or from the Actions tab on a branch with no open pull request into development. |
||||
| Hydra gates | ✅ |
Quality workflow — 2026-10-15 19:20 UTC
Download the full PDF report from the workflow artifacts.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Decisions 239 and 247 to 249: the special rights
agent_useandllm_useare enforced where they apply,object_publishanddashboard_vieware retired, and only an administrator or the organisation's owner can change who holds a special right.OpenSpec change:
openspec/changes/enforce-organisation-special-rights/(proposal, design with the file and line of every enforcement point, spec delta, tasks).What is enforced, and where
agent_useChatService::processMessage()(lib/Service/ChatService.php:253), reached byPOST /api/chat/sendandPOST /api/chat/stream;ChatStreamController::stream()(:302) before the first heartbeat;McpAgentScope::loadAgent()(lib/Service/Mcp/McpAgentScope.php:248) for MCPinitialize,tools/listandtools/callllm_useChatService::processMessage()andChatStreamController::stream(). Every language model call in the app (answer, title, summary) is reached only fromprocessMessage()Semantic and hybrid search embed the query with an embedding model and do not need
llm_use(decision 249); they are unchanged.The one reader is still
OrganisationSpecialRightsfrom #4601. It gainsmayUse()(administrators pass,*means every signed-in user, a key the organisation never set does not narrow, a named organisation that cannot be loaded refuses, no organisation and no default organisation does not narrow) andopenToEveryoneInEveryOrganisation().A refused chat returns 403 from
/api/chat/send, aforbiddenframe naming the right from/api/chat/stream(its headers are already sent, as for a conversation that is not yours), and a JSON-RPC error from MCP. Nothing is stored and no model is called.Retired:
object_publishanddashboard_view(decisions 247, 248)Neither was ever enforced: objects have had no publish action since migration
Version1Date20260313130000, and/api/dashboard/*is administrator only. Both are removed from the organisation's defaults, the editor's Special Rights tab, the frontend entity, the admin-group backfill, the upgrade step and the docs. An organisation that still stores them loads and is served unchanged; nothing reads them and nothing writes them.Who changes special rights
PUT/PATCH /api/organisations/{uuid}refuses a change toagent_use,llm_useorgrant_managefrom anybody but an administrator or the owner (403 withrightsnaming them). #4601 did this forgrant_manageonly. A member still edits the rest of the organisation. The Special Rights tab is read-only for them and says why, and offers "Everyone" (*).Upgrade notes: what becomes refused for whom
Nothing, on the day of the upgrade. Before, every signed-in user could chat and run agents whatever the lists said, and the lists said
[]or["admin"]on most organisations. The post-migration repair stepOpenSpecialRightsToEveryoneadds*beside the stored groups underagent_useandllm_usein every organisation, once (app configspecial_rights_opened_to_everyone), and never touchesgrant_manageor the retired keys. A new organisation starts with["*"]under both.After that, taking
*out of a list refuses users outside its groups, and a later upgrade does not put it back.object_publishanddashboard_viewwere never enforced, so retiring them refuses nobody; they disappear from the editor, and values already stored are ignored.Verified
Run once in the foreground on this branch:
phpcs0,phpmd(withHOMEset to an empty dir, both rule sets, wholelib/) 0 and 0,psalm --threads=10 (no errors),phpstan0phpunit --no-coverage: the 328 tests around this change pass (Rbac, chat, MCP, organisation controller and model, repair step, OrganisationService). The full Unit suite on the previous commit: 26,183 tests, 15 errors, allOC\DB\Schema\Table is not loadable(13 AuditTrail and 2 DraftRemovalAuditRow tests need an installed Nextcloud)src/entities/organisation4/4,npm run lint0 (warnings only; the editor's ellipsis suppression count went down from 8 to 6 with the two removed rows), prettier 0,test:l10nOKNot verified live: no instance was used. Both endpoints are plain
Controllers, notOCSControllers, so OCSMiddleware leaves the 403 as it is; the tests assert that, and drive the real controller over the realChatServiceand the real reader.Inherited: the 15 PHPUnit errors above; 942 eslint warnings, none in the changed lines.